Skip to content

Conversation

diemol
Copy link
Member

@diemol diemol commented Oct 15, 2025

User description

🔗 Related Issues

Fixes #16299

💥 What does this PR do?

See the linked issue for more details.

This PR fetches the browser_protocol file, retrieves all the includes, fetches them, and concatenates the files into a single browser_protocol.pdl file, allowing the rest of the process to continue.


PR Type

Bug fix


Description

  • Implements fetching and concatenation of Chrome DevTools Protocol domain files

  • Adds flatten_browser_pdl() function to resolve include directives in browser_protocol.pdl

  • Fetches individual domain .pdl files from Chromium repository and merges them

  • Integrates flattening step into the CDP update workflow


Diagram Walkthrough

flowchart LR
  A["Fetch browser_protocol.pdl"] --> B["Parse include directives"]
  B --> C["Fetch domain .pdl files"]
  C --> D["Concatenate domains"]
  D --> E["Overwrite browser_protocol.pdl"]
Loading

File Walkthrough

Relevant files
Bug fix
update_cdp.py
Implement browser_protocol.pdl flattening for Chromium file structure

scripts/update_cdp.py

  • Added flatten_browser_pdl() function to parse and fetch included
    domain files
  • Extracts include directives using regex pattern matching
  • Fetches domain files from Chromium repository and concatenates them
  • Integrated flattening step into add_pdls() workflow after initial
    fetch
+21/-0   

@selenium-ci selenium-ci added the B-build Includes scripting, bazel and CI integrations label Oct 15, 2025
Copy link
Contributor

qodo-merge-pro bot commented Oct 15, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Unvalidated remote write

Description: The code fetches and writes remote content directly to the local protocol file without
validating response status, size, or content type, which could allow corrupted or
malicious data to be persisted if the upstream or network is compromised.
update_cdp.py [75-81]

Referred Code
for domain_file in includes:
    url = base_url + domain_file
    response = http.request("GET", url)
    concatenated += response.data.decode("utf-8") + "\n"
# Overwrite the file with concatenated domains
with open(file_path, "w") as file:
    file.write(concatenated)
Ticket Compliance
🎫 No ticket provided
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Copy link
Contributor

qodo-merge-pro bot commented Oct 15, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Add error handling for requests

Add a status check after the HTTP request to ensure it was successful (status
200), and raise an exception if it failed to prevent generating a corrupted
file.

scripts/update_cdp.py [77-78]

 response = http.request("GET", url)
+if response.status != 200:
+    raise Exception(f"Failed to fetch {url}, status code: {response.status}")
 concatenated += response.data.decode("utf-8") + "\n"
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies missing error handling for a network request, which could lead to silent failures and corrupted files. Implementing this check improves the script's robustness.

Medium
Learned
best practice
Properly close HTTP responses

Ensure the HTTP response is closed after use to avoid leaking connections; use a
context manager with preload_content=False or explicitly release it.

scripts/update_cdp.py [77-78]

-response = http.request("GET", url)
-concatenated += response.data.decode("utf-8") + "\n"
+response = http.request("GET", url, preload_content=False)
+try:
+    concatenated += response.read().decode("utf-8") + "\n"
+finally:
+    response.release_conn()
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Enforce deterministic and safe resource handling by closing HTTP responses and files via context managers.

Low
  • Update

@diemol diemol merged commit 8221fd1 into trunk Oct 15, 2025
18 checks passed
@diemol diemol deleted the building_cdp_again branch October 15, 2025 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: Bazel build stops working from Chromium 141

3 participants